[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
[LAA] Fix type mismatch in getStartAndEndForAccess.#183116
Conversation
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Kshitij Paranjape (kshitijvp) Changes
Running This is caused by a type mismatch between Full diff: https://github.com/llvm/llvm-project/pull/183116.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/LoopAccessAnalysis.cpp b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
index 057636050db25..67530ac4ba31d 100644
--- a/llvm/lib/Analysis/LoopAccessAnalysis.cpp
+++ b/llvm/lib/Analysis/LoopAccessAnalysis.cpp
@@ -250,8 +250,13 @@ static bool evaluatePtrAddRecAtMaxBTCWillNotWrap(
return true;
});
if (DerefRK) {
- DerefBytesSCEV =
- SE.getUMaxExpr(DerefBytesSCEV, SE.getSCEV(DerefRK.IRArgValue));
+ const SCEV *DerefRKSCEV = SE.getSCEV(DerefRK.IRArgValue);
+ // Ensure both operands have the same type
+ Type *CommonTy =
+ SE.getWiderType(DerefBytesSCEV->getType(), DerefRKSCEV->getType());
+ DerefBytesSCEV = SE.getNoopOrAnyExtend(DerefBytesSCEV, CommonTy);
+ DerefRKSCEV = SE.getNoopOrAnyExtend(DerefRKSCEV, CommonTy);
+ DerefBytesSCEV = SE.getUMaxExpr(DerefBytesSCEV, DerefRKSCEV);
}
if (DerefBytesSCEV->isZero())
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll b/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll
new file mode 100644
index 0000000000000..0a28775ca290e
--- /dev/null
+++ b/llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll
@@ -0,0 +1,34 @@
+; RUN: opt -S -p loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output 2>&1 < %s | FileCheck %s
+; REQUIRES: asserts
+
+; CHECK-LABEL: LV: Checking a loop in 'test_assumed_bounds_type_mismatch'
+; CHECK: LV: Not vectorizing: Cannot vectorize uncountable loop.
+
+define void @test_assumed_bounds_type_mismatch(ptr noalias %array, ptr readonly %pred, i32 %n) nosync nofree {
+entry:
+ %n_bytes = mul nuw nsw i32 %n, 2
+ call void @llvm.assume(i1 true) [ "dereferenceable"(ptr %pred, i32 %n_bytes) ]
+ %tc = sext i32 %n to i64
+ br label %for.body
+
+for.body:
+ %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.inc ]
+ %st.addr = getelementptr inbounds i16, ptr %array, i64 %iv
+ %data = load i16, ptr %st.addr, align 2
+ %inc = add nsw i16 %data, 1
+ store i16 %inc, ptr %st.addr, align 2
+ %ee.addr = getelementptr inbounds i16, ptr %pred, i64 %iv
+ %ee.val = load i16, ptr %ee.addr, align 2
+ %ee.cond = icmp sgt i16 %ee.val, 500
+ br i1 %ee.cond, label %exit, label %for.inc
+
+for.inc:
+ %iv.next = add nuw nsw i64 %iv, 1
+ %counted.cond = icmp eq i64 %iv.next, %tc
+ br i1 %counted.cond, label %exit, label %for.body
+
+exit:
+ ret void
+}
+
+declare void @llvm.assume(i1)
\ No newline at end of file
|
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
| @@ -0,0 +1,34 @@ | |||
| ; RUN: opt -S -p loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output 2>&1 < %s | FileCheck %s | |||
There was a problem hiding this comment.
you'd need to use -passes=print<aceess-info> for tests in this directory.
would be good to add to other existing early exit tests
There was a problem hiding this comment.
I have added to the existing one
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
|
ping |
| DerefBytesSCEV = SE.getNoopOrAnyExtend(DerefBytesSCEV, CommonTy); | ||
| DerefRKSCEV = SE.getNoopOrAnyExtend(DerefRKSCEV, CommonTy); |
There was a problem hiding this comment.
Why any extend? Both quantities should be positive, so better always use Zext?
| ret void | ||
| } | ||
|
|
||
| define void @test_assumed_bounds_type_mismatch(ptr noalias %array, ptr readonly %pred, i32 %n) nosync nofree { |
There was a problem hiding this comment.
is there a reason we need a loop with a store? If not, would be good to remove the store and add to llvm/test/Transforms/LoopVectorize/dereferenceable-info-from-assumption-variable-size.ll. It should get vectorized with the crash fixed
There was a problem hiding this comment.
yes the store is redundant in this case, needs to be removed.
There was a problem hiding this comment.
removing the store instruction is causing faulting load, instead of being vectorized.
llvm/test/Transforms/LoopVectorize/early_exit_store_legality.ll
Outdated
Show resolved
Hide resolved
The test will not be vectorized as it is potentially an early exit loop due to and removing the store instruction causes faulting load. |
It's fine as is. I also went ahead and clarified the PR title, hope that's OK with you |
|
Thanks, no issues with the PR title. |
sure |
|
I think you do not need to put complete backtrace in the commit message, we do not do it in practice, describing the issue and solution is enough. |
Sure, will change. |
`SE.getUMaxExpr` causes assertion failure due to type mismatch here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/LoopAccessAnalysis.cpp#L253 Running `opt -S -p loop-vectorize -debug-only=loop-vectorize llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.ll ` without the changes made in LoopAccessAnalysis.cpp causes assertion failure. Attaching the stack dump for reference: ``` LV: Checking a loop in 'loop_contains_store_assumed_bounds' from input.ll LV: Loop hints: force=? width=4 interleave=0 LV: Found a loop: for.body LV: Found an induction variable. opt: /home/kshitij/llvm-project/llvm/lib/Analysis/ScalarEvolution.cpp:3918: const llvm::SCEV* llvm::ScalarEvolution::getMinMaxExpr(llvm::SCEVTypes, llvm::SmallVectorImpl<const llvm::SCEV*>&): Assertion `getEffectiveSCEVType(Ops[i]->getType()) == ETy && "Operand types don't match!"' failed. PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug. Stack dump: 0. Program arguments: opt -S -passes=loop-vectorize -debug-only=loop-vectorize -force-vector-width=4 -disable-output input.ll 1. Running pass "function(loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>)" on module "input.ll" 2. Running pass "loop-vectorize<no-interleave-forced-only;no-vectorize-forced-only;>" on function "loop_contains_store_assumed_bounds" #0 0x000058ee97c5e652 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/usr/local/bin/opt+0x4f44652) llvm#1 0x000058ee97c5af0f llvm::sys::RunSignalHandlers() (/usr/local/bin/opt+0x4f40f0f) llvm#2 0x000058ee97c5b05c SignalHandler(int, siginfo_t*, void*) Signals.cpp:0:0 llvm#3 0x00007c49d4c45330 (/lib/x86_64-linux-gnu/libc.so.6+0x45330) llvm#4 0x00007c49d4c9eb2c __pthread_kill_implementation ./nptl/pthread_kill.c:44:76 llvm#5 0x00007c49d4c9eb2c __pthread_kill_internal ./nptl/pthread_kill.c:78:10 llvm#6 0x00007c49d4c9eb2c pthread_kill ./nptl/pthread_kill.c:89:10 llvm#7 0x00007c49d4c4527e raise ./signal/../sysdeps/posix/raise.c:27:6 llvm#8 0x00007c49d4c288ff abort ./stdlib/abort.c:81:7 llvm#9 0x00007c49d4c2881b _nl_load_domain ./intl/loadmsgcat.c:1177:9 llvm#10 0x00007c49d4c3b517 (/lib/x86_64-linux-gnu/libc.so.6+0x3b517) llvm#11 0x000058ee98003fdb llvm::ScalarEvolution::getMinMaxExpr(llvm::SCEVTypes, llvm::SmallVectorImpl<llvm::SCEV const*>&) (/usr/local/bin/opt+0x52e9fdb) llvm#12 0x000058ee98004507 llvm::ScalarEvolution::getUMaxExpr(llvm::SCEV const*, llvm::SCEV const*) (/usr/local/bin/opt+0x52ea507) llvm#13 0x000058ee980dc728 llvm::getStartAndEndForAccess(llvm::Loop const*, llvm::SCEV const*, llvm::Type*, llvm::SCEV const*, llvm::SCEV const*, llvm::ScalarEvolution*, llvm::DenseMap<std::pair<llvm::SCEV const*, llvm::Type*>, std::pair<llvm::SCEV const*, llvm::SCEV const*>, llvm::DenseMapInfo<std::pair<llvm::SCEV const*, llvm::Type*>, void>, llvm::detail::DenseMapPair<std::pair<llvm::SCEV const*, llvm::Type*>, std::pair<llvm::SCEV const*, llvm::SCEV const*>>>*, llvm::DominatorTree*, llvm::AssumptionCache*, std::optional<llvm::ScalarEvolution::LoopGuards>&) (/usr/local/bin/opt+0x53c2728) llvm#14 0x000058ee9814008b llvm::isDereferenceableAndAlignedInLoop(llvm::LoadInst*, llvm::Loop*, llvm::ScalarEvolution&, llvm::DominatorTree&, llvm::AssumptionCache*, llvm::SmallVectorImpl<llvm::SCEVPredicate const*>*) (/usr/local/bin/opt+0x542608b) llvm#15 0x000058ee9a0fa1ca llvm::LoopVectorizationLegality::canUncountableExitConditionLoadBeMoved(llvm::BasicBlock*) (/usr/local/bin/opt+0x73e01ca) llvm#16 0x000058ee9a0faee0 llvm::LoopVectorizationLegality::isVectorizableEarlyExitLoop() (/usr/local/bin/opt+0x73e0ee0) llvm#17 0x000058ee9a104678 llvm::LoopVectorizationLegality::canVectorize(bool) (/usr/local/bin/opt+0x73ea678) llvm#18 0x000058ee9a08c953 llvm::LoopVectorizePass::processLoop(llvm::Loop*) (/usr/local/bin/opt+0x7372953) llvm#19 0x000058ee9a090e21 llvm::LoopVectorizePass::runImpl(llvm::Function&) (/usr/local/bin/opt+0x7376e21) llvm#20 0x000058ee9a0914e0 llvm::LoopVectorizePass::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/opt+0x73774e0) llvm#21 0x000058ee99e419a5 llvm::detail::PassModel<llvm::Function, llvm::LoopVectorizePass, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) PassBuilderPipelines.cpp:0:0 llvm#22 0x000058ee97f18905 llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) (/usr/local/bin/opt+0x51fe905) llvm#23 0x000058ee995d70d5 llvm::detail::PassModel<llvm::Function, llvm::PassManager<llvm::Function, llvm::AnalysisManager<llvm::Function>>, llvm::AnalysisManager<llvm::Function>>::run(llvm::Function&, llvm::AnalysisManager<llvm::Function>&) AMDGPUTargetMachine.cpp:0:0 llvm#24 0x000058ee97f17051 llvm::ModuleToFunctionPassAdaptor::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/opt+0x51fd051) llvm#25 0x000058ee995d7775 llvm::detail::PassModel<llvm::Module, llvm::ModuleToFunctionPassAdaptor, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) AMDGPUTargetMachine.cpp:0:0 llvm#26 0x000058ee97f1783d llvm::PassManager<llvm::Module, llvm::AnalysisManager<llvm::Module>>::run(llvm::Module&, llvm::AnalysisManager<llvm::Module>&) (/usr/local/bin/opt+0x51fd83d) llvm#27 0x000058ee9c153909 llvm::runPassPipeline(llvm::StringRef, llvm::Module&, llvm::TargetMachine*, llvm::TargetLibraryInfoImpl*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::ToolOutputFile*, llvm::StringRef, llvm::ArrayRef<llvm::PassPlugin>, llvm::ArrayRef<std::function<void (llvm::PassBuilder&)>>, llvm::opt_tool::OutputKind, llvm::opt_tool::VerifierKind, bool, bool, bool, bool, bool, bool, bool, bool) (/usr/local/bin/opt+0x9439909) llvm#28 0x000058ee97c3f380 optMain (/usr/local/bin/opt+0x4f25380) llvm#29 0x00007c49d4c2a1ca __libc_start_call_main ./csu/../sysdeps/nptl/libc_start_call_main.h:74:3 llvm#30 0x00007c49d4c2a28b call_init ./csu/../csu/libc-start.c:128:20 llvm#31 0x00007c49d4c2a28b __libc_start_main ./csu/../csu/libc-start.c:347:5 llvm#32 0x000058ee97c309a5 _start (/usr/local/bin/opt+0x4f169a5) ``` This is caused by a type mismatch between `SE.getSCEV(DerefRK.IRArgValue)` and `DerefBytesSCEV`. Fixing this by extending them to the wider type.
SE.getUMaxExprcauses assertion failure due to type mismatch here:https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/LoopAccessAnalysis.cpp#L253
Running
opt -S -p loop-vectorize -debug-only=loop-vectorize llvm/test/Analysis/LoopAccessAnalysis/type-mismatch-in-scalar-evolution.llwithout the changes made in LoopAccessAnalysis.cpp causes assertion failure.This is caused by a type mismatch between
SE.getSCEV(DerefRK.IRArgValue)andDerefBytesSCEV.Fixing this by extending them to the wider type.